Skip to content

UrlFlow and errorFlow are initialized with the state value and eagerly started#6688

Merged
TimoPtr merged 2 commits intomainfrom
fix/url_flow_value
Apr 13, 2026
Merged

UrlFlow and errorFlow are initialized with the state value and eagerly started#6688
TimoPtr merged 2 commits intomainfrom
fix/url_flow_value

Conversation

@TimoPtr
Copy link
Copy Markdown
Member

@TimoPtr TimoPtr commented Apr 8, 2026

Summary

From #6687, the FrontendViewModel.urlFlow and errorFlow are always returning null if it doesn't have any subscriber and invoking .value on it. It is because of the stateIn WhileSubscribe behavior instead we should initialize with the state behavior and always collect to update this field. It is important since we are using .value in few places so we need to make sure it reflect the current state (We could still have a race but regarding of where it is used I think we are fine).

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Copilot AI review requested due to automatic review settings April 8, 2026 11:40
Copilot AI reviewed Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts FrontendViewModel’s derived urlFlow and errorFlow so their .value reflects the current viewState even when there are no active subscribers, addressing a stateIn(WhileSubscribed) behavior where .value could remain null.

Changes:

  • Switch urlFlow and errorFlow to stateIn(..., SharingStarted.Eagerly, initialValueFromState) instead of WhileSubscribed.
  • Update unit tests to assert on errorFlow.value directly (without needing to collect first) and add a derived-flow test group.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModel.kt Makes derived flows eagerly started and initializes them from the current viewState to keep .value current
app/src/test/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModelTest.kt Updates tests around derived flows and removes reliance on “activate via subscription” for errorFlow

Comment thread app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModel.kt
Comment thread app/src/test/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModelTest.kt Outdated
@jpelgrom
Copy link
Copy Markdown
Member

jpelgrom commented Apr 11, 2026

From #6687, the FrontendViewModel.urlFlow and errorFlow are always returning null if it doesn't have any subscriber and invoking .value on it (...) It is important since we are using .value in few places so we need to make sure it reflect the current state

I can understand this change and why it might be needed for race conditions, but not why you're referring to #6687 as that doesn't touch either of these two flows?

@TimoPtr
Copy link
Copy Markdown
Member Author

TimoPtr commented Apr 11, 2026

From #6687, the FrontendViewModel.urlFlow and errorFlow are always returning null if it doesn't have any subscriber and invoking .value on it (...) It is important since we are using .value in few places so we need to make sure it reflect the current state

I can understand this change and why it might be needed for race conditions, but not why you're referring to #6687 as that doesn't touch either of these two flows?

I found the issue while working on the #6687 because my first implementation was using the urlFlow to give the current url to the bridge before I switch to the Uistate.

@jpelgrom
Copy link
Copy Markdown
Member

jpelgrom commented Apr 11, 2026

I found the issue while working on the #6687 because my first implementation was using the urlFlow to give the current url to the bridge before I switch to the Uistate.

Do you want to switch back to using the urlFlow after this is merged?

@TimoPtr
Copy link
Copy Markdown
Member Author

TimoPtr commented Apr 13, 2026

I found the issue while working on the #6687 because my first implementation was using the urlFlow to give the current url to the bridge before I switch to the Uistate.

Do you want to switch back to using the urlFlow after this is merged?

No I don't think it is useful since we already have the UIState that is the source of truth, the main goal of urlFlow is for the error screen.

@TimoPtr TimoPtr merged commit a6dfb2d into main Apr 13, 2026
24 checks passed
@TimoPtr TimoPtr deleted the fix/url_flow_value branch April 13, 2026 08:50
@TimoPtr TimoPtr added the WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav. label Apr 22, 2026
@abb848 abb848 mentioned this pull request Apr 27, 2026
Open
48 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@jpelgrom jpelgrom jpelgrom approved these changes

Assignees

No one assigned

Labels

cla-signed WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TimoPtr @jpelgrom